-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution][Detections] Implements indicator match rule cypress test #84323
Conversation
@elasticmachine merge upstream |
Just a suggestion but es_archiver does support non gzip of data. Looking at: x-pack/test/security_solution_cypress/es_archives/threat_data/data.json.gz That looks like it only has a single record within it, so I would just not gzip it up and check it in as ~/projects/kibana/x-pack/test/security_solution_cypress/es_archives/threat_data/mappings.json I would trim the mappings down if possible to only match the parts of the data set you need to match. Technically everything will work out ok if we use ECS compatible mappings even if they don't have the full data set including the full mapping. Also nothing wrong with adding something to the mappings that don't exist in ECS as long as we don't use a conflicting field name from ECS. For example, on backend e2e tests I am using very small data sets and small mappings like these recent ones: The only "required field" is But in my above tests it shouldn't matter since I only use a Optional here, do what's best, just mentioning this and depends on what you're testing. I typically these days try to keep the least amount of mapping/data for functional tests since ECS is constantly evolving. For other tests or issues with backwards compatibility with ECS I try to do the same thing and just concentrate on types such as keyword/wildcard, etc... and not directly test something large or the actual fields of ECS if I don't have to. Although I might need to do more of those later if we have bugs directly related to different ECS mapping types. However, even then I try to abstract it away into something that doesn't directly involve ECS mappings if I can (other than That also usually catches bugs if someone is relying a field to exist from ECS as an added bonus since all those fields should be optional other than |
"type": "index", | ||
"value": { | ||
"aliases": { | ||
"playing": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you want this here where the alias is playing
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that, thanks @FrankHassanabad :)
return alertCount > 0; | ||
}); | ||
}, | ||
{ interval: 500, timeout: 12000 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a reason for this to be here previously, i.e. timing reasons? Are other tests relying on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method is on the same file but displayed in a different order. I just sorted the methods alphabetically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see parameters for interval and timeout in the other method. Were the values here used at all?
@FrankHassanabad I think I have simplified and fix the data:
Can you please review the PR again, lots of thanks :) |
@elasticmachine merge upstream |
const alertCount = parseInt(countText, 10) || 0; | ||
return alertCount > 0; | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
For the backend e2e, I started just like this, but then later I had to add a specific number to wait for a specific number of alerts as even with things like refresh=wait_for
within the code there does exist some time before the Nth alert shows up.
I had some flakiness where 1 signal would show up on the backend but I needed to wait for like 2 or 3. No changes requested, but if we see flaky we will probably have to add that integer here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the tests here for exceptions. Users use exceptions A LOT and we are fixing bugs/refactoring parts of it so this is going to be so great for us to ensure that the exceptions are all operating as expected.
💛 Build succeeded, but was flaky
Test FailuresChrome X-Pack UI Functional Tests.x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout·ts.Actions and Triggers app create alert should show save confirmation before creating alert with no actionsStandard Out
Stack Trace
Metrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
…ss test (elastic#84323) * implemnts indicator match rule cypress test * fixes merge issue * fixes type check issues * fixes mapping * simplifies data * fixes excpetions flakiness * fixes alerts test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* master: (40 commits) fix: 🐛 don't add separator befor group on no main items (elastic#83166) [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323) [APM] Add APM agent config options (elastic#84678) Fixed a11y issue on rollup jobs table selection (elastic#84567) [Discover] Refactor getContextUrl to separate file (elastic#84503) [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654) [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749) [Lens] Migrate legacy es client and remove total hits as int (elastic#84340) Improve logging pipeline in @kbn/legacy-logging (elastic#84629) Catch @hapi/podium errors (elastic#84575) [Discover] Unskip date histogram test (elastic#84727) Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791) [Enterprise Search] Fix schema errors button (elastic#84842) [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589) [Maps] Always initialize routes on server-startup (elastic#84806) [Fleet] EPM support to handle uploaded file paths (elastic#84708) [Snapshot Restore] Fix initial policy form state (elastic#83928) Upgrade Node.js to version 14 (elastic#83425) [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992) [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006) ...
* master: (236 commits) fix: 🐛 don't add separator befor group on no main items (elastic#83166) [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323) [APM] Add APM agent config options (elastic#84678) Fixed a11y issue on rollup jobs table selection (elastic#84567) [Discover] Refactor getContextUrl to separate file (elastic#84503) [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654) [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749) [Lens] Migrate legacy es client and remove total hits as int (elastic#84340) Improve logging pipeline in @kbn/legacy-logging (elastic#84629) Catch @hapi/podium errors (elastic#84575) [Discover] Unskip date histogram test (elastic#84727) Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791) [Enterprise Search] Fix schema errors button (elastic#84842) [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589) [Maps] Always initialize routes on server-startup (elastic#84806) [Fleet] EPM support to handle uploaded file paths (elastic#84708) [Snapshot Restore] Fix initial policy form state (elastic#83928) Upgrade Node.js to version 14 (elastic#83425) [Security Solution] Keep Endpoint policies up to date with license changes (elastic#83992) [Security Solution][Exceptions] Implement exceptions for ML rules (elastic#84006) ...
…overy-action-group * upstream/master: (48 commits) [Lens] accessibility screen reader issues (elastic#84395) [Logs UI] Fetch single log entries via a search strategy (elastic#81710) fix: 🐛 don't add separator befor group on no main items (elastic#83166) [Security Solution][Detections] Implements indicator match rule cypress test (elastic#84323) [APM] Add APM agent config options (elastic#84678) Fixed a11y issue on rollup jobs table selection (elastic#84567) [Discover] Refactor getContextUrl to separate file (elastic#84503) [Embeddable] Export CSV action for Lens embeddables in dashboard (elastic#83654) [TSVB] [Cleanup] Remove extra dateFormat props (elastic#84749) [Lens] Migrate legacy es client and remove total hits as int (elastic#84340) Improve logging pipeline in @kbn/legacy-logging (elastic#84629) Catch @hapi/podium errors (elastic#84575) [Discover] Unskip date histogram test (elastic#84727) Rename server.xsrf.whitelist to server.xsrf.allowlist (elastic#84791) [Enterprise Search] Fix schema errors button (elastic#84842) [APM] Removes react-sticky dependency in favor of using CSS (elastic#84589) [Maps] Always initialize routes on server-startup (elastic#84806) [Fleet] EPM support to handle uploaded file paths (elastic#84708) [Snapshot Restore] Fix initial policy form state (elastic#83928) Upgrade Node.js to version 14 (elastic#83425) ...
…ss test (#84323) (#84882) * implemnts indicator match rule cypress test * fixes merge issue * fixes type check issues * fixes mapping * simplifies data * fixes excpetions flakiness * fixes alerts test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
In this PR we are adding a new test in order to check that an Indicator Match rule can be created and is able to populate alerts.
For this test we have created two different archives:
The
threat_data
archive contains an auditbeat type document that matches withthreat-data-*
index pattern.The
threat_index
archive, contains two documents that matches withthreat-index-*
index pattern that contains the threats, that matches with thethreat-data
.The test: